Skip to content

KeyPass milestone 1 delivery#5

Merged
oshakarishvili merged 3 commits intoPolkadot-Fast-Grants:masterfrom
uliana1one:master
Jun 29, 2025
Merged

KeyPass milestone 1 delivery#5
oshakarishvili merged 3 commits intoPolkadot-Fast-Grants:masterfrom
uliana1one:master

Conversation

@uliana1one
Copy link

📬 Milestone Delivery Checklist

  • The delivery template has been copied and updated.
  • This pull request is being made by the same account as the accepted application.
  • The milestone was completed within 3 months of approval.
  • The milestone is complete (no partial deliveries).
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • The delivery is according to the delivery guidelines.
  • I understand that the milestone will be evaluated within 14 days, and if changes are needed, I will have one opportunity to fix and resubmit within 14 days.

Link to the application pull request: Polkadot-Fast-Grants/apply#22

Copy link

@oshakarishvili oshakarishvili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @uliana1one, great work! Approved from my end.

@oshakarishvili oshakarishvili requested review from a team, platonicsocrates and sacha-l June 10, 2025 10:24
@sacha-l
Copy link
Collaborator

sacha-l commented Jun 11, 2025

I particularly enjoyed the attention to detail in the docs and examples. Great work, well done.

A couple things that came up in my review:

  1. Running the Dockers gave me errors, I had to modify your dockerfiles to make it run smoothly
  2. The tutorial is nice but it doesn't help me as a newbie to the library to get started easily. Could you update it so that we follow concrete steps and provide boilerplate project structure if necessary?
  3. I don't see any address transforming from Ethereum to Substrate in the library. I was expecting this to be part of the basic SDK? I would think that working with public key addresses directly would be a lot easier (EVM addresses right-padded to 32 bytes to fit Substrate's AccountId32 address type) than using the SS58 as your public key starting point.

If you can address 2 and update it so I can finish my review this is approved on my end!

@uliana1one
Copy link
Author

Hey @sacha-l!
I have updated the deliverable template (and the deliverable) to address the comments.

For 1, I am not sure which exact issues you run into when running docker. It all works for me when I run it from a clean state but I would love to know what went wrong for you so I can fix it.

For 2, I included more straightforward instructions and a couple boilerplate apps so you can see how it is intended to work even if the tutorials aren’t helpful enough. Please, let me know if anything is still unclear. I have also recorded a demo showing the boilerplate and authentication in action.

For the third point, I actually considered using right-padded EVM addresses for Substrate, but I went with native SS58 addresses instead for a few practical reasons:

  1. Wallet Compatibility: I found that most Substrate wallets (like Polkadot.js and Talisman) work with SS58 addresses, and I wanted to make sure the SDK works smoothly with existing wallet infrastructure.
  2. Security: SS58's built-in checksum validation is really helpful for catching address typos, which I'm using in the validation functions.
  3. Chain Flexibility: I noticed that not all Substrate chains use AccountId32, so I went with SS58 to keep things more flexible.
    I ended up implementing a DID-based approach for cross-chain identity instead of direct address transformation. But I'm totally open to adding address transformation utilities if you think that would be more useful!

Let me know if there are any other questions or adjustments needed!

Copy link
Author

@uliana1one uliana1one left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated delivery doc

@oshakarishvili oshakarishvili requested a review from dimdomsum June 19, 2025 15:23
Copy link

@platonicsocrates platonicsocrates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@sacha-l sacha-l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for addressing my comments and making the updates. Looking forward to reviewing milestone 2.

@oshakarishvili oshakarishvili merged commit 87d5563 into Polkadot-Fast-Grants:master Jun 29, 2025
@oshakarishvili
Copy link

Hey @uliana1one, the payout was made 3 days ago: https://polkadot.subsquare.io/treasury/child-bounties/63_3. Thanks for your patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants